[Lens as code] Fix temporal scale on x axis and histogram brush#264134
[Lens as code] Fix temporal scale on x axis and histogram brush#264134markov00 merged 9 commits intoelastic:mainfrom
Conversation
|
/sync-ci |
Catch flakiness early (recommended)Recommended before merge: run the flaky test runner against this PR to catch flakiness early. This PR adds a new Scout test ( Trigger a run with the Flaky Test Runner UI or post this comment on the PR: This check is experimental. Share your feedback in the #appex-qa channel. Posted via Macroscope — Flaky Test Runner nudge |
There was a problem hiding this comment.
Scout Test Review
Found 2 issues (1 major, 1 minor). See inline comments for details.
This review is experimental. Share your feedback in the #appex-qa channel.
Posted via Macroscope — Scout Test Review
There was a problem hiding this comment.
Scout Test Review
Found 1 new issue (1 minor). 2 prior findings (template-literal test names, login in test body) still apply — see earlier review comments.
This review is experimental. Share your feedback in the #appex-qa channel.
Posted via Macroscope — Scout Test Review
| const chartStatus = chart.locator('.echChartStatus'); | ||
| const debugJson = await chartStatus.getAttribute('data-ech-debug-state'); |
There was a problem hiding this comment.
🔵 Leverage Playwright auto-waiting (best practices)
getAttribute is not a web-first assertion — it resolves immediately and won't retry if the attribute hasn't been set yet. Although the render-complete check above largely mitigates this, reading the value before the retrying toHaveAttribute assertion means debugJson could be null in a race, leading to a confusing failure in assertTemporalXAxis instead of the clear "attribute missing" message.
Swap the order so the retrying assertion runs first:
const chartStatus = chart.locator('.echChartStatus');
+await expect(chartStatus, 'Elastic Charts debug state attribute missing').toHaveAttribute(
+ 'data-ech-debug-state'
+);
const debugJson = await chartStatus.getAttribute('data-ech-debug-state');
-await expect(chartStatus, 'Elastic Charts debug state attribute missing').toHaveAttribute(
- 'data-ech-debug-state'
-);
const debug = JSON.parse(debugJson ?? '{}') as DebugState;Posted via Macroscope — Scout Test Review
Scout Test ReviewFound 0 new issues. Prior feedback (explicit test names, login in This review is experimental. Share your feedback in the #appex-qa channel. Posted via Macroscope — Scout Test Review |
|
Pinging @elastic/kibana-visualizations (Team:Visualizations) |
stratoula
left a comment
There was a problem hiding this comment.
Just some code moving to a new reusable function, LGTM!
andrimal
left a comment
There was a problem hiding this comment.
Tested locally and both issues are fixed ✨
|
Starting backport for target branches: 9.4 https://github.com/elastic/kibana/actions/runs/24678575176 |
💚 All backports created successfully
Note: Successful backport PRs will be merged automatically after passing CI. Questions ?Please refer to the Backport tool documentation |
…#264134) (#264482) # Backport This will backport the following commits from `main` to `9.4`: - [[Lens as code] Fix temporal scale on x axis and histogram brush (#264134)](#264134) <!--- Backport version: 9.6.6 --> ### Questions ? Please refer to the [Backport tool documentation](https://github.com/sorenlouv/backport) <!--BACKPORT [{"author":{"name":"Marco Vettorello","email":"marco.vettorello@elastic.co"},"sourceCommit":{"committedDate":"2026-04-20T16:40:37Z","message":"[Lens as code] Fix temporal scale on x axis and histogram brush (#264134)\n\n## Summary\n\nAPI generated charts currently doesn't contain information if a XY chart\nis actually a time based chart or a categorical chart.\nWe have left this information to be computed/decided at runtime (as it's\ncurrently done when you create a chart from the Lens UI).\nBut information about the current timeField is required:\n- to filter the chart in the configured time range\n- to property assign the X axis scale type\n- to enable the timeseries brushing\n\nThis PR fixes this problem by correctly picking the timeField when\nnecessary and clear the dataview cache that was polluted with the\ninitial adhoc dataview without timefield.\n\nRight after ESQL query is executed, I'm adjusting the x scale to time if\nthe table column layer type is date.\n\nFor ESQL folks, I've just extracted the getESQLTimeField into its own\nfile, so it can be easily reused without the need to touch the DataViews\nas it was initially used.\n\n\n\nfix https://github.com/elastic/kibana/issues/262425\nfix https://github.com/elastic/kibana/issues/262181","sha":"fb3adeae36ecd2d5c416213bd9bdfb11a5410816","branchLabelMapping":{"^v9.5.0$":"main","^v(\\d+).(\\d+).\\d+$":"$1.$2"}},"sourcePullRequest":{"labels":["Team:Visualizations","release_note:skip","backport:version","v9.4.0","v9.5.0"],"title":"[Lens as code] Fix temporal scale on x axis and histogram brush","number":264134,"url":"https://github.com/elastic/kibana/pull/264134","mergeCommit":{"message":"[Lens as code] Fix temporal scale on x axis and histogram brush (#264134)\n\n## Summary\n\nAPI generated charts currently doesn't contain information if a XY chart\nis actually a time based chart or a categorical chart.\nWe have left this information to be computed/decided at runtime (as it's\ncurrently done when you create a chart from the Lens UI).\nBut information about the current timeField is required:\n- to filter the chart in the configured time range\n- to property assign the X axis scale type\n- to enable the timeseries brushing\n\nThis PR fixes this problem by correctly picking the timeField when\nnecessary and clear the dataview cache that was polluted with the\ninitial adhoc dataview without timefield.\n\nRight after ESQL query is executed, I'm adjusting the x scale to time if\nthe table column layer type is date.\n\nFor ESQL folks, I've just extracted the getESQLTimeField into its own\nfile, so it can be easily reused without the need to touch the DataViews\nas it was initially used.\n\n\n\nfix https://github.com/elastic/kibana/issues/262425\nfix https://github.com/elastic/kibana/issues/262181","sha":"fb3adeae36ecd2d5c416213bd9bdfb11a5410816"}},"sourceBranch":"main","suggestedTargetBranches":["9.4"],"targetPullRequestStates":[{"branch":"9.4","label":"v9.4.0","branchLabelMappingKey":"^v(\\d+).(\\d+).\\d+$","isSourceBranch":false,"state":"NOT_CREATED"},{"branch":"main","label":"v9.5.0","branchLabelMappingKey":"^v9.5.0$","isSourceBranch":true,"state":"MERGED","url":"https://github.com/elastic/kibana/pull/264134","number":264134,"mergeCommit":{"message":"[Lens as code] Fix temporal scale on x axis and histogram brush (#264134)\n\n## Summary\n\nAPI generated charts currently doesn't contain information if a XY chart\nis actually a time based chart or a categorical chart.\nWe have left this information to be computed/decided at runtime (as it's\ncurrently done when you create a chart from the Lens UI).\nBut information about the current timeField is required:\n- to filter the chart in the configured time range\n- to property assign the X axis scale type\n- to enable the timeseries brushing\n\nThis PR fixes this problem by correctly picking the timeField when\nnecessary and clear the dataview cache that was polluted with the\ninitial adhoc dataview without timefield.\n\nRight after ESQL query is executed, I'm adjusting the x scale to time if\nthe table column layer type is date.\n\nFor ESQL folks, I've just extracted the getESQLTimeField into its own\nfile, so it can be easily reused without the need to touch the DataViews\nas it was initially used.\n\n\n\nfix https://github.com/elastic/kibana/issues/262425\nfix https://github.com/elastic/kibana/issues/262181","sha":"fb3adeae36ecd2d5c416213bd9bdfb11a5410816"}}]}] BACKPORT--> Co-authored-by: Marco Vettorello <marco.vettorello@elastic.co>
The lens entry bundle was 72-83B over its 86,000B page-load limit after rebasing this branch on main. Nothing in this PR's scope (kbn-unified-chart-section-viewer + one Discover test spec) can reach lens's bundle — a codebase-wide grep shows zero imports of @kbn/unified-chart-section-viewer from x-pack/platform/plugins/shared/lens, and a local dist build's stats.json contains no matching module paths. Cumulative drift from recent Lens-as-code commits that merged into main (e.g., elastic#262871, elastic#264134, elastic#261581, elastic#264147, elastic#263810) pushed the entry bundle just over the line. Raising the limit here is the documented path when no in-PR contribution is found. Limit set via `node scripts/build_kibana_platform_plugins --focus lens --update-limits`.
Summary
API generated charts currently doesn't contain information if a XY chart is actually a time based chart or a categorical chart.
We have left this information to be computed/decided at runtime (as it's currently done when you create a chart from the Lens UI).
But information about the current timeField is required:
This PR fixes this problem by correctly picking the timeField when necessary and clear the dataview cache that was polluted with the initial adhoc dataview without timefield.
Right after ESQL query is executed, I'm adjusting the x scale to time if the table column layer type is date.
For ESQL folks, I've just extracted the getESQLTimeField into its own file, so it can be easily reused without the need to touch the DataViews as it was initially used.
fix #262425
fix #262181